StepTHn: do a pre lookup for the bin using a uniform grid#15444
Conversation
|
@jgrosseo with these two I gain 10% in performance for the Correlation business. On hyperloop the results seem to be compatible, for what I can tell. Can you validate and eventually merge? |
|
I can look into this next week... |
Replaces 4 virtual calls to GetAt / SetAt / AddAt with just one to updateBin.
Rather than doing a O(log N) binary search for the correct bin, precompute a uniform index for a O(1) lookup and find the actual bin using a short linear search.
|
@jgrosseo: I ran your script and did a comparison for both #15443 and this one. The simple one (#15443) which simply gets rid of the repeated dynamic dispatch resulted in identical AnalysisRoot.root files (and by consequence, also dphi_corr.root files are identical). This one was a bit more tricky, because a few bins were differing because of some rounding errors due slightly different bin finding logic. This was fixed and the logic is now exactly the same as TAxis::FindBin. The results are now identical, both for I attach a PDF with the validation: dphi_comparison.pdf for this PR. I think both PRs are ready to be merged. |
|
Thanks for the extensive validation. Fingers crossed :) |
|
Yes, and it's monday, so we have a full week to fix it... :-D |
|
discussed privately and merged. |
2 participants
Rather than doing a O(log N) binary search for the correct bin, precompute a uniform index for a O(1) lookup and find the actual bin using a short linear search.
Stack created with Sapling. Best reviewed with ReviewStack.